Skip to content

fix: StorageResult last_update_block non-optional, add integration test#917

Merged
3alpha merged 1 commit intomainfrom
fix/storage-response-flags-last-update-block
Apr 9, 2026
Merged

fix: StorageResult last_update_block non-optional, add integration test#917
3alpha merged 1 commit intomainfrom
fix/storage-response-flags-last-update-block

Conversation

@3alpha
Copy link
Copy Markdown
Collaborator

@3alpha 3alpha commented Apr 9, 2026

Usage related changes

  • StorageResult.last_update_block is now non-optional: When INCLUDE_LAST_UPDATE_BLOCK response flag is set on starknet_getStorageAt, the last_update_block field is now always returned as a u64 (defaulting to 0 for never-modified slots) instead of Option<u64>. This aligns with the RPC spec which requires the field in the STORAGE_RESULT schema.

Development related changes

  • Added get_storage_with_response_flags integration test covering:
    • Untouched storage slot returns last_update_block = 0
    • last_update_block correctly tracks the block of each storage update across multiple transfers and empty block advancements
    • Response without flags deserializes as plain Value variant
    • Response with INCLUDE_LAST_UPDATE_BLOCK deserializes as ValueWithMetadata variant
    • Both variants return the same underlying storage value

Checklist:

  • Checked out the contribution guidelines
  • Applied formatting - ./scripts/format.sh
  • No linter errors - ./scripts/clippy_check.sh
  • No unused dependencies - ./scripts/check_unused_deps.sh
  • No spelling errors - ./scripts/check_spelling.sh
  • Performed code self-review
  • Rebased to the latest commit of the target branch (or merged it into my branch)
    • Once you make the PR reviewable, please avoid force-pushing
  • Updated the docs if needed - ./website/README.md
  • Linked the issues resolvable by this PR - linking info
  • Updated the tests if needed; all passing - execution info

Summary by CodeRabbit

  • Bug Fixes
    • Updated the storage retrieval endpoint to consistently return block information in its response when the appropriate flag is enabled, rather than potentially omitting it in certain scenarios.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 9, 2026

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key(s) in object: 'auto_resolve_threads'
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 45579f1d-d645-4b93-ad3f-8d1a5f530723

📥 Commits

Reviewing files that changed from the base of the PR and between c126758 and d51d1cd.

📒 Files selected for processing (3)
  • crates/starknet-devnet-server/src/api/endpoints.rs
  • crates/starknet-devnet-server/src/api/models/mod.rs
  • tests/integration/old_state.rs

Walkthrough

This PR modifies the storage response handling to ensure last_update_block is always present as a concrete u64 value (defaulting to 0) when the IncludeLastUpdateBlock response flag is set. The change involves updating the StorageResult model to make last_update_block non-optional, adjusting the endpoint logic to unwrap the value with a default, and adding a comprehensive integration test that validates storage retrieval behavior with and without response flags across multiple blocks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

enhancement

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main changes: making StorageResult.last_update_block non-optional and adding an integration test.
Description check ✅ Passed The description comprehensively covers usage and development changes, includes completed checklist items, though one item (linking issues) remains unchecked.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/storage-response-flags-last-update-block

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@3alpha
Copy link
Copy Markdown
Collaborator Author

3alpha commented Apr 9, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 9, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@3alpha 3alpha merged commit 9f091c9 into main Apr 9, 2026
6 checks passed
@3alpha 3alpha deleted the fix/storage-response-flags-last-update-block branch April 9, 2026 18:25
@3alpha 3alpha added bug Something isn't working testing Related to code testing labels Apr 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working testing Related to code testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant